Skip to content

feat: Implement recaptcha verification #2349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 12 commits into
base: development
Choose a base branch
from
Open

feat: Implement recaptcha verification #2349

wants to merge 12 commits into from

Conversation

@atm1504
Copy link
Member

@atm1504 atm1504 commented Aug 16, 2019

Fixes #2348 , #2319

Changes:

Screenshots for the change:
Will be updated once complete

@atm1504 atm1504 changed the title [WIP]: Implement recaptcha verification feat: Implement recaptcha verification Aug 19, 2019
@auto-label auto-label bot added the feature label Aug 19, 2019
@atm1504
Copy link
Member Author

atm1504 commented Aug 21, 2019

@iamareebjamal should we have this feature. This was implemented as discussed in the Gitter channel, regarding the changes in the server-side script.
But, @liveHarshit shared some comments fossasia/open-event-attendee-android#1943 (comment) where it was discussed that ReCaptcha won't be needed in the android clients. Can you please confirm what should I do?

@iamareebjamal
Copy link
Member

iamareebjamal commented Aug 22, 2019

Discuss this with SUSI server guys

@atm1504
Copy link
Member Author

atm1504 commented Aug 22, 2019

Discuss this with SUSI server guys

We had a discussion regarding this in the gitter channel, they said to implement recaptcha.

@atm1504
Copy link
Member Author

atm1504 commented Aug 23, 2019

@iamareebjamal please review it. Also, please tell where to store the API key.

@ci-reporter
Copy link

ci-reporter bot commented Aug 24, 2019 •
edited
Loading

The build is failing

✨ Good work on this PR so far! ✨ Unfortunately, the Circle CI build is failing as of 3ff7a72. Here's the output:

lint check
> Task :app:preBuild UP-TO-DATE

I'm sure you can fix it! If you need help, don't hesitate to ask a maintainer of the project!

Failed build for de75047
lint check
> Task :app:preBuild UP-TO-DATE
Failed build for 21ed178
lint check
> Task :app:preBuild UP-TO-DATE
This comment was automagically generated by ci-reporter. If you see a problem, open an issue here.

@ci-reporter
Copy link

ci-reporter bot commented Aug 24, 2019

The build is failing

✨ Good work on this PR so far! ✨ Unfortunately, the Circle CI build is failing as of 21ed178. Here's the output:

lint check
> Task :app:preBuild UP-TO-DATE

I'm sure you can fix it! If you need help, don't hesitate to ask a maintainer of the project!

This comment was automagically generated by ci-reporter. If you see a problem, open an issue here.

@mariobehling
Copy link
Member

mariobehling commented Aug 29, 2019

Please fix Travis.

@mariobehling
Copy link
Member

mariobehling commented Aug 29, 2019

Please provide a screenshot.

mariobehling
mariobehling requested changes Aug 29, 2019
View reviewed changes
Copy link
Member

@mariobehling mariobehling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as in the other PR: We cannot keep the API key in the app itself. This would be a security issue. It should be added during building of the app using Travis environment variables. Please implement it in that way.

@atm1504 atm1504 mentioned this pull request Sep 19, 2019
Open
@mkmar01
Copy link
Member

mkmar01 commented Feb 7, 2020

@atm1504 please refer to the way youtube API key is stored in the App.

@mkmar01
Copy link
Member

mkmar01 commented Feb 7, 2020

This PR is open for a long time, if anyone wants to take this up, they can

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mariobehling mariobehling mariobehling requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add ReCaptcha for second login attempt

4 participants

@atm1504 @iamareebjamal @mariobehling @mkmar01